Added user attributes, client auth resources, service account roles #104

AndrewChubatiuk · 2019-04-26T11:26:01Z

No description provided.

…rm-provider-keycloak into client-resources

AndrewChubatiuk · 2019-04-30T21:14:56Z

@mrparkers
Hello again. Prepared a PR with some changes

Added user attributes
Added client and client authorization policy datasources
Added client authorization resources (resource, permission, scope)
Added service account roles resource
Added examples of how to use it

mrparkers

Thanks for the contributions!

I'm okay with the addition of all of these resources and data sources in one PR, but I'm a little uneasy about also upgrading Keycloak to 6.0.1 in the same PR. Would you be okay with opening a smaller PR that only contains the changes needed for the 6.0.1 upgrade? I can merge that one first, and you can rebase this PR with master to continue with these changes.

Also, since this is the first time data sources will be supported (thank you for this by the way), we may need to make some significant changes to the file names. The convention for all of the other Terraform providers I've seen prefixes all resources with resource_ and all data sources with data_. I'm not sure why I didn't do this from the start 😢. Would you be okay with opening a separate PR that contains only those changes, as well as the new data sources? I'm afraid this PR will be massive if we make all of the file name changes here.

mrparkers · 2019-05-03T14:02:01Z

keycloak/group.go

 }

 func (keycloakClient *KeycloakClient) ListGroupsWithName(realmId, name string) ([]*Group, error) {
 var groups []*Group

- err := keycloakClient.get(fmt.Sprintf("/realms/%s/groups?search=%s", realmId, url.QueryEscape(name)), &groups)
+ err := keycloakClient.get(fmt.Sprintf("/realms/%s/groups?search=%s", realmId, url.QueryEscape(name)), &groups, nil)


Since you added the params argument to the get method, would you mind updating this line to use that argument for the search param here?

mrparkers · 2019-05-03T14:03:25Z

keycloak/openid_client.go


- err := keycloakClient.get(fmt.Sprintf("/realms/%s/clients?clientId=%s", realmId, clientId), &clients)
+ err := keycloakClient.get(fmt.Sprintf("/realms/%s/clients?clientId=%s", realmId, clientId), &clients, nil)


Could you update this line to use the new params argument for the clientId param?

mrparkers · 2019-05-03T14:04:18Z

keycloak/user.go

 }

 func (keycloakClient *KeycloakClient) GetUserByUsername(realmId, username string) (*User, error) {
 var users []*User

- err := keycloakClient.get(fmt.Sprintf("/realms/%s/users?username=%s", realmId, url.QueryEscape(username)), &users)
+ err := keycloakClient.get(fmt.Sprintf("/realms/%s/users?username=%s", realmId, url.QueryEscape(username)), &users, nil)


Could you update this line to use the new params argument for the username param?

mrparkers · 2019-05-03T14:05:10Z

provider/data_source_keycloak_openid_client.go

+)
+
+func dataSourceKeycloakOpenidClient() *schema.Resource {
+ return &schema.Resource{


Could the schema for the openid_client resource be re-used here instead of defining it again?

I assume the answer is no since the data attributes will almost all be Computed, but I want to hear your thoughts on this.

Right, now the only difference is that data source attributes are computed, but usually data source scheme has bigger differences. I would like to leave it as it is

mrparkers · 2019-05-03T14:57:53Z

keycloak/openid_client.go

+type OpenidClientAuthorizationSettings struct {
+ PolicyEnforcementMode string `json:"policyEnforcementMode,omitempty"`
+ AllowRemoteResourceManagement bool `json:"allowRemoteResourceManagement,omitempty"`
+}


I think these new structs and methods can probably be moved to their own files, unless there is a specific reason you wanted them here.

mrparkers · 2019-05-03T15:16:48Z

provider/keycloak_openid_client_test.go

@@ -129,6 +129,10 @@ func TestAccKeycloakOpenidClient_updateInPlace(t *testing.T) {
 directAccessGrantsEnabled := randomBool()
 serviceAccountsEnabled := randomBool()

+ if !standardFlowEnabled {
+ implicitFlowEnabled = !standardFlowEnabled
+ }


What's the reason for this change?

At least standard or implicit flow should be enabled to set valid redirect URI

Is that new behavior with version 5.0.0 or 6.0.0 of Keycloak. I realize that there is no point in setting valid redirect uris without either of these two flows enabled, but I don't believe Keycloak 4.8.3 requires it.

I'm fine with keeping this change, I was just curious what the context was.

mrparkers · 2019-05-03T15:18:38Z

provider/keycloak_openid_client.go

+ serviceAccountUserId = serviceAccountUser.Id
+ }
+
+ setOpenidClientData(data, client, serviceAccountUserId)


This block of code (lines 301 - 311) seems to be repeated multiple times. Could you update the setOpenidClientData to do this instead?

mrparkers · 2019-05-03T15:22:05Z

keycloak/openid_client.go

+ ContainerId: clientId,
+ ServiceAccountUserId: serviceAccountUserId,
+ })
+ err := keycloakClient.get(fmt.Sprintf("/realms/%s/users/%s/role-mappings/clients/%s", realm, serviceAccountUserId, clientId), &serviceAccountRoles, nil)


I'm not sure I understand what's going on here. It looks like you're initializing the serviceAccountRoles slice with a single element, but then it's immediately being overwritten by this line. Is there a reason for this?

mrparkers · 2019-05-03T15:27:05Z

keycloak/openid_client.go

+
+func (keycloakClient *KeycloakClient) DeleteOpenidClientServiceAccountRole(realm, serviceAccountUserId, clientId, roleId string) error {
+ serviceAccountRoles := []OpenidClientServiceAccountRole{}
+ serviceAccountRoles = append(serviceAccountRoles, OpenidClientServiceAccountRole{


Same comment as above.

mrparkers · 2019-05-03T15:27:30Z

keycloak/openid_client.go

+ }
+ serviceAccountRole.Id = role.Id
+ serviceAccountRoles := []OpenidClientServiceAccountRole{}
+ serviceAccountRoles = append(serviceAccountRoles, *serviceAccountRole)


Do you need to use append here? I think you could just initialize the slice with the single element instead of making it empty and adding to it.

…r-keycloak into client-resources

…rm-provider-keycloak into client-resources

AndrewChubatiuk · 2019-05-10T15:31:39Z

@mrparkers
Fixed all your PR comments and added tests.

mrparkers

Looks good, thanks for the continued contributions!

Andrew Chubatiuk and others added 12 commits April 25, 2019 18:24

initial commit

3dd442f

go fmt

6909b20

fixed resource body params

853524d

fixed resource id

5555d95

upgraded keycloak and added example

bcbd78b

minor fix

81d5740

fixed resources

fcce373

fixed web origins

1e41445

added openid client authorization resources

2c1c57b

resolved merge conflicts

78f84e9

added examples

a96715e

Merge branch 'client-resources' of github.com:AndrewChubatiuk/terrafo…

50d0dd4

…rm-provider-keycloak into client-resources

AndrewChubatiuk changed the title ~~Client resources~~ Added user attributes, client auth resources, service account roles Apr 29, 2019

Andrii Chubatiuk added 10 commits April 30, 2019 00:44

updated endpoints

ae21d62

minor fix

2ca6644

fixed examples

6ca699f

removed unused util function

558218b

minor fix

8edfe09

enable authorization service

ea1eae2

minor fix

1b25743

fixed tests for keycloak 6

cb87711

minor fix

5b540dd

fixed tests

9131e7f

Andrii Chubatiuk added 6 commits May 1, 2019 00:18

minor fix

fdfe893

minor fix

ed8a631

minor fix

62d0e1b

updated service account role logic

49697d0

added resource_server_id

3f5ec97

updated examples

4c1606c

resolved conflicts

1a1a724

mrparkers requested changes May 3, 2019

View reviewed changes

Andrii Chubatiuk and others added 22 commits May 9, 2019 17:37

Merge branch 'master' of github.com:AndrewChubatiuk/terraform-provide…

0e38f7c

…r-keycloak into client-resources

Merge branch 'master' into client-resources

ff257fe

renamed new providers, PR comments fixes

cdcd08a

Merge branch 'client-resources' of github.com:AndrewChubatiuk/terrafo…

e3c4562

…rm-provider-keycloak into client-resources

fmt fix

eb3ea2a

pr comments

484e7c2

added tests

0084dd8

minor fix

13fa442

minor fix

51edd05

minor test fix

5f7328c

fixed getclientbyName

a1fd8bc

fixed delete request

3cd9b3d

Merge branch 'client-resources' of github.com:AndrewChubatiuk/terrafo…

288a4a7

…rm-provider-keycloak into client-resources

fixed auth policy test

adf1a3b

test

eda5f46

fixed service account tests

26567d3

fixed typo

69a5c4f

minor fix

0cdf594

minor update

c9ef71b

minor fix

517452b

fixed delete request for service account role

fae15f0

minor fix

d7c0e21

Andrii Chubatiuk added 2 commits May 11, 2019 00:30

trigger build

8919a4e

added default resource removal

a688cc0

mrparkers approved these changes May 14, 2019

View reviewed changes

mrparkers merged commit 1267c01 into mrparkers:master May 14, 2019

AndrewChubatiuk deleted the client-resources branch May 14, 2019 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added user attributes, client auth resources, service account roles #104

Added user attributes, client auth resources, service account roles #104

AndrewChubatiuk commented Apr 26, 2019

AndrewChubatiuk commented Apr 30, 2019

mrparkers left a comment

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

mrparkers May 3, 2019

AndrewChubatiuk May 10, 2019

AndrewChubatiuk commented May 10, 2019

mrparkers left a comment


		err := keycloakClient.get(fmt.Sprintf("/realms/%s/clients?clientId=%s", realmId, clientId), &clients)
		err := keycloakClient.get(fmt.Sprintf("/realms/%s/clients?clientId=%s", realmId, clientId), &clients, nil)

Added user attributes, client auth resources, service account roles #104

Added user attributes, client auth resources, service account roles #104

Conversation

AndrewChubatiuk commented Apr 26, 2019

AndrewChubatiuk commented Apr 30, 2019

mrparkers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndrewChubatiuk commented May 10, 2019

mrparkers left a comment

Choose a reason for hiding this comment